[Python] Add a couple quality-of-life improvemenets to `testing.util.assert_that` #30771

hjtran · 2024-03-27T18:23:37Z

These changes just extend assert_that to accommodate a couple of common footguns I've seen.

assert_that will now check to see if you're calling it with a pipeline that has already been run. This catches the following circumstance:

with beam.Pipeline() as p:
    my_pcoll = p | # ...
assert_that(my_pcoll, equal_to([1,2,3]) # should've been indented

assert_that automatically creates a unique label if the label it's given is already taken. Usually when writing unit tests, we don't really need very specific labels for assertions. When this comes up, we usually have to just manually increment every assertion which is quite tedious.

github-actions · 2024-03-27T19:05:37Z

Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers

hjtran · 2024-03-27T19:25:44Z

assign set of reviewers

hjtran · 2024-03-27T19:26:20Z

I ran the linting instructions here:
https://cwiki.apache.org/confluence/display/BEAM/Spotless+pre-commit

But they had no effect on any files

github-actions · 2024-03-27T19:26:53Z

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @liferoad for label python.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

hjtran · 2024-03-27T20:04:03Z

Turns out I jumped the gun and there are more than just lint failures. There were a few true tests failures that came up from actual tests that weren't actually using assert_that correct. I fixed a few but all the groupby example tests weren't trivial to fix. I confirmed that if I changed the assertion in those tests, they would've always passed before the changes in this PR

codecov · 2024-03-27T22:11:12Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.44%. Comparing base (827e96d) to head (ef4829d).
Report is 113 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #30771       +/-   ##
===========================================
+ Coverage   38.52%   71.44%   +32.92%     
===========================================
  Files         698      710       +12     
  Lines      102371   104823     +2452     
===========================================
+ Hits        39440    74894    +35454     
+ Misses      61301    28299    -33002     
  Partials     1630     1630

Flag	Coverage Δ
python	`81.23% <100.00%> (+52.08%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

liferoad · 2024-03-27T22:57:19Z

Turns out I jumped the gun and there are more than just lint failures. There were a few true tests failures that came up from actual tests that weren't actually using assert_that correct. I fixed a few but all the groupby example tests weren't trivial to fix. I confirmed that if I changed the assertion in those tests, they would've always passed before the changes in this PR

So what is the plan here? close this PR for now or you want to keep working on this?

hjtran · 2024-03-27T23:00:00Z

I pushed test fixes and skips so we should be all set

…

On Wed, Mar 27, 2024, 6:57 PM liferoad ***@***.***> wrote: Turns out I jumped the gun and there are more than just lint failures. There were a few true tests failures that came up from actual tests that weren't actually using assert_that correct. I fixed a few but all the groupby example tests weren't trivial to fix. I confirmed that if I changed the assertion in those tests, they would've always passed before the changes in this PR So what is the plan here? close this PR for now or you want to keep working on this? — Reply to this email directly, view it on GitHub <#30771 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACQJSFHDQR6P6VV2NLMJNM3Y2NFGJAVCNFSM6AAAAABFLM7DIOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRUGEYTSOBZGE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

hjtran · 2024-03-27T23:21:09Z

I created a spinoff issue since I think fixing the tests (which were already broken before these changes) is probably out of scope

github-actions · 2024-08-19T13:08:53Z

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.

damccorm · 2024-08-19T13:10:20Z

Looks like this is very close to crossing the finish line - @hjtran any chance you can take another look at this?

hjtran · 2024-08-26T13:43:42Z

Yes, I'll try to take another round of looks at all the PRs I have up this week. I haven't been able to get a reliable dev environment going so every round of iteration has been a bit tough

tvalentyn · 2024-08-26T18:46:36Z

I haven't been able to get a reliable dev environment going so every round of iteration has been a bit tough

Oh that's not good. We have https://s.apache.org/beam-python-dev-wiki that might help, or if not we can definitely edit the instructions there.

Some people previously wrote and used https://github.com/apache/beam/blob/master/local-env-setup.sh specifically for this purpose (getting a working environment fast), but I am not sure if the script has enough usage, possibly it hasn't been maintained and I haven't used it recently.

hjtran · 2024-08-27T14:12:45Z

Yeah I used that wiki article quite a bit and also used local-env-setup.sh. I should've written more notes on what went wrong as I've forgotten now. There was some kind of go installation issue when I tried to use the container setup and then some kind of dependency issue when I tried to setup a local environment. There are a few threads in the dev@ mailing list I think where I mentioned these things.

I'll give it another go sometime soon and report back

github-actions · 2024-09-04T12:14:10Z

Reminder, please take a look at this pr: @damccorm

hjtran · 2024-09-20T14:49:59Z

@damccorm finally got a consistent dev environment up. I updated this branch according to @robertwb 's last comment. Hopefully it loos good now

damccorm

Thanks!

…assert_that` (apache#30771) * Add tests for qol changes * implement qols * revert local start-build-env.sh change * Fix or skip tests * undo start-build-env.sh change again * format * revert global_aggregate change * add missing paren * Update groupby_test.py * Update groupby_test.py * address a couple nits/comments * add in pipeline = actual.pipeline since it is actually used elsewhere * Update sdks/python/apache_beam/testing/util.py Note about update compatibility being the reason for not doing this ubiquitously. * comment out asserts * use early returns instead of comments * Use global boolean for early returns in groupby_test --------- Co-authored-by: Robert Bradshaw <[email protected]>

hjtran added 3 commits March 27, 2024 14:16

Add tests for qol changes

81aafbb

implement qols

28df2c1

revert local start-build-env.sh change

09d78ed

github-actions bot added the python label Mar 27, 2024

github-actions bot added the Next Action: Reviewers label Mar 27, 2024

Fix or skip tests

e078f77

github-actions bot added examples and removed examples labels Mar 27, 2024

undo start-build-env.sh change again

6dd363b

github-actions bot added examples and removed examples labels Mar 27, 2024

hjtran mentioned this pull request Mar 27, 2024

[Failing Test]: GroupBy example tests (groupby_test.py) fail #30778

Open

16 tasks

format

ae4cee4

github-actions bot added examples and removed examples labels Mar 27, 2024

revert global_aggregate change

2240f56

github-actions bot added examples and removed examples labels Mar 27, 2024

add missing paren

ef4829d

github-actions bot added examples and removed examples labels Mar 28, 2024

github-actions bot added stale and removed stale labels Jun 19, 2024

github-actions bot added the stale label Aug 19, 2024

github-actions bot removed the stale label Aug 20, 2024

github-actions bot added Next Action: Reviewers and removed Next Action: Author labels Aug 26, 2024

github-actions bot added the slow-review label Sep 4, 2024

damccorm added Next Action: Author and removed Next Action: Reviewers slow-review labels Sep 4, 2024

Merge branch 'master' into testing_qols

9f841fa

github-actions bot added examples Next Action: Reviewers and removed examples Next Action: Author labels Sep 20, 2024

Use global boolean for early returns in groupby_test

c70c8fc

github-actions bot added examples and removed examples labels Sep 20, 2024

damccorm approved these changes Sep 21, 2024

View reviewed changes

damccorm merged commit 6a09545 into apache:master Sep 21, 2024
94 checks passed

damccorm mentioned this pull request Sep 21, 2024

Move assert into pipeline #32530

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Python] Add a couple quality-of-life improvemenets to `testing.util.assert_that` #30771

[Python] Add a couple quality-of-life improvemenets to `testing.util.assert_that` #30771

hjtran commented Mar 27, 2024

github-actions bot commented Mar 27, 2024

hjtran commented Mar 27, 2024

hjtran commented Mar 27, 2024

github-actions bot commented Mar 27, 2024

hjtran commented Mar 27, 2024

codecov bot commented Mar 27, 2024 •

edited

Loading

liferoad commented Mar 27, 2024

hjtran commented Mar 27, 2024 via email

hjtran commented Mar 27, 2024

github-actions bot commented Aug 19, 2024

damccorm commented Aug 19, 2024

hjtran commented Aug 26, 2024

tvalentyn commented Aug 26, 2024

hjtran commented Aug 27, 2024

github-actions bot commented Sep 4, 2024

hjtran commented Sep 20, 2024

damccorm left a comment

[Python] Add a couple quality-of-life improvemenets to testing.util.assert_that #30771

[Python] Add a couple quality-of-life improvemenets to testing.util.assert_that #30771

Conversation

hjtran commented Mar 27, 2024

github-actions bot commented Mar 27, 2024

hjtran commented Mar 27, 2024

hjtran commented Mar 27, 2024

github-actions bot commented Mar 27, 2024

hjtran commented Mar 27, 2024

codecov bot commented Mar 27, 2024 • edited Loading

Codecov Report

liferoad commented Mar 27, 2024

hjtran commented Mar 27, 2024 via email

hjtran commented Mar 27, 2024

github-actions bot commented Aug 19, 2024

damccorm commented Aug 19, 2024

hjtran commented Aug 26, 2024

tvalentyn commented Aug 26, 2024

hjtran commented Aug 27, 2024

github-actions bot commented Sep 4, 2024

hjtran commented Sep 20, 2024

damccorm left a comment

Choose a reason for hiding this comment

[Python] Add a couple quality-of-life improvemenets to `testing.util.assert_that` #30771

[Python] Add a couple quality-of-life improvemenets to `testing.util.assert_that` #30771

codecov bot commented Mar 27, 2024 •

edited

Loading